Improve converters #613
Improve converters #613
Conversation
antoine-aumjaud
commented
Feb 6, 2015
antoine-aumjaud
commented
- fix NPE when value is null for all converters or when string is empty
- create a generic array converter (works for all arrays), use underlying converter for elements
- create a generic collections converter (works for all collections), use underlying converter for elements
- fix NPE when value is null for all converters or when string is empty - create a generic array converter (works for all arrays), use underlying converter for elements - create a generic collections converter (works for all collections), use underlying converter for elements
|
I've not found the changeLog.txt
|
|
CONTRIBUTING.md is not up to date. I always update the page FitNesse.ReleaseNotes. That page is also included in the distribution, which is a lot nicer. |
…mprovements Improve converters
| } | ||
|
|
||
| public String fromString(String arg) { | ||
| return arg; | ||
| return !StringUtils.isBlank(arg) ? arg : null; |
There was a problem hiding this comment.
I think this is the offending line for #686.
|
Yes, that's correct. I have changed the behavior of String converter to have the same logic for all converters: empty string set object reference to null. For the issue #686, @davidmagill could override this StringConverter in ConverterRegistry. The real issue is that we can't set an empty string to a String field. |
|
I can do a pull request to add this feature. |
|
We can't make a distinction between |
|
Personally, I prefer to keep the consistency instead of the backward compatibility. |
|
I think having a flag is a minimum requirement for this. I know that the Overriding the behavior by adding your own StringConverter is something We also need to advertise this heavily in the release notes. This will On Tue, Mar 24, 2015 at 4:34 AM, Antoine Aumjaud notifications@github.com
|
|
Yes @woodybrood, we would have added it in the release note. My bad, I have not indicated this in the PR comment. |
|
I did not notice it as well, among all the improvements. I think a switch is somewhat hard, since we also have to pass it on to the SUT. Shall I apply #687 for now? |
|
I think you can apply your PR. It 's better for backward compatibility. |
